-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Kinetic energy fraction #329
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, Benoit! It looks good and I also added an additional test, comparing to my other code, and it passes.
I am just wondering if it is not more practical for the reader to just return the energy budget rather than the efficiency factor. That would just mean we multiply the efficiencyFactor with 3/4 * alpha *enthalpy/energy.
I think this is more practical, as the energy budget goes into the formula for gravitational waves. The efficiencyFactor still has to be converted and that is a possible source for confusion.
Could you also add a formula in the doc string?
I was a bit unsure about how to compute K because the energy, which enters in its calculation, is not always well-defined since you can always add a constant to it. If I understand correctly, the usual convention is to set the vacuum energy to 0 inside the bubble. But with a general potential, there is not always a clear separation between the vacuum and thermal energy, so I don't know what it means to set the vacuum energy to 0. Do you know a better definition? |
That is a good point, I hadn't thought of it. But indeed, that's also the only definition that I know. OK, then I think it's best to leave the responsibility with the user. |
Agreed! |
I added a function in Hydrodynamics and HydrodynamicsTemplateModel to compute the efficiency factor for a given wall velocity. I also made a test to compare the results obtained with both methods.
I get a DeprecationWarning in the test, but I really don't know why. Pytest doesn't give much detail about it and it only appears in the test, so I'm not sure what to do about that.